Simplify trivial constants in SimplifyConstCondition #147654

dianqk · 2025-10-14T00:25:34Z

After InstSimplify-after-simplifycfg with -Zub_checks=false, there are many of the following patterns.

_13 = const false;
assume(copy _13);
_12 = unreachable_unchecked::precondition_check() -> [return: bb1, unwind unreachable];

Simplifying them to unreachable early should make CFG simpler.

dianqk · 2025-10-14T00:54:20Z

@bors try @rust-timer queue

Simplify trivial constants in SimplifyConstCondition

rust-bors · 2025-10-14T03:06:20Z

☀️ Try build successful (CI)
Build commit: 2d02757 (2d0275706b38d9e50cb9131ae9dc3ae769529a20, parent: 4b94758d2ba7d0ef71ccf5fde29ce4bc5d6fe2a4)

rust-timer · 2025-10-14T04:17:58Z

Finished benchmarking commit (2d02757): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.2%	[-0.4%, -0.1%]	9
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 0.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	4.9%	[4.9%, 4.9%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.6%	[-3.6%, -3.6%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.7%	[-3.6%, 4.9%]	2

Cycles

Results (secondary 3.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.0%	[2.5%, 3.5%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 0.0%, secondary 0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.2%]	7
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	2
Improvements ✅ (primary)	-0.1%	[-0.1%, -0.0%]	8
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[-0.1%, 0.2%]	15

Bootstrap: 475.312s -> 475.552s (0.05%)
Artifact size: 388.16 MiB -> 388.14 MiB (-0.00%)

dianqk · 2025-10-14T13:44:27Z

@bors2 try parent=last @rust-timer queue

Simplify trivial constants in SimplifyConstCondition

rustbot · 2025-10-14T13:49:52Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

rustbot · 2025-10-14T13:49:55Z

r? @davidtwco

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rust-bors · 2025-10-14T15:55:26Z

☀️ Try build successful (CI)
Build commit: 2b77d79 (2b77d79571afb4b51d2e238b5007f06b01912318, parent: 4b94758d2ba7d0ef71ccf5fde29ce4bc5d6fe2a4)

rust-timer · 2025-10-14T18:03:34Z

Finished benchmarking commit (2b77d79): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.2%	[-0.3%, -0.1%]	9
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 0.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	4.7%	[4.7%, 4.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.0%	[-3.0%, -3.0%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.9%	[-3.0%, 4.7%]	2

Cycles

Results (secondary 3.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.5%	[3.5%, 3.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 0.0%, secondary 0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.2%]	7
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	2
Improvements ✅ (primary)	-0.1%	[-0.1%, -0.0%]	8
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[-0.1%, 0.2%]	15

Bootstrap: 475.312s -> 474.926s (-0.08%)
Artifact size: 388.16 MiB -> 388.14 MiB (-0.00%)

cjgillot · 2025-10-15T03:13:35Z

compiler/rustc_mir_transform/src/simplify_branches.rs

        let mut patch = MirPatch::new(body);

        'blocks: for (bb, block) in body.basic_blocks.iter_enumerated() {
+            let mut pre_local_const: Option<(Local, &'_ ConstOperand<'_>)> = None;


Why only locals and not any non-indirect place?
Do you mind adding a comment explaining this is safe as it only lives for a single statement.

An indirect place should also be safe. I cannot find a real-world case for non-locals by running ./x build --stage 2, but checking place makes code simpler.

cjgillot · 2025-10-15T03:15:05Z

compiler/rustc_mir_transform/src/simplify_branches.rs

+                } else if let StatementKind::Assign(box (ref lhs, ref rvalue)) = stmt.kind
+                    && let Some(local) = lhs.as_local()
+                    && let Rvalue::Use(Operand::Constant(c)) = rvalue
+                    && c.const_.ty().is_bool()


Why is this condition useful?

cjgillot · 2025-10-15T03:15:41Z

compiler/rustc_mir_transform/src/simplify_branches.rs

+                        c
+                    } else {
+                        continue;
+                    };


Can we use something like this in terminators too?

compiler/rustc_mir_transform/src/lib.rs

cjgillot · 2025-10-15T03:23:10Z

compiler/rustc_mir_transform/src/gvn.rs

+            TerminatorKind::SwitchInt { .. }
+                | TerminatorKind::Goto { .. }
+                | TerminatorKind::Unreachable
        );


Should we make this match exhaustive?

rust-log-analyzer · 2025-10-15T15:42:14Z

The job aarch64-gnu-llvm-20-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

diff of stderr:

2   --> $DIR/collect-in-promoted-const.rs:11:19
3    |
4 LL |     const C: () = panic!();
-    |                   ^^^^^^^^ evaluation of `Fail::<T>::C` failed here
- 
- note: erroneous constant encountered
-   --> $DIR/collect-in-promoted-const.rs:22:21
-    |
- LL |         let _val = &Fail::<T>::C;
-    |                     ^^^^^^^^^^^^
- 
- error[E0080]: evaluation panicked: explicit panic
-   --> $DIR/collect-in-promoted-const.rs:11:19
-    |
- LL |     const C: () = panic!();
17    |                   ^^^^^^^^ evaluation of `Fail::<i32>::C` failed here
18 
19 note: erroneous constant encountered

21    |
22 LL |         let _val = &Fail::<T>::C;
23    |                     ^^^^^^^^^^^^
-    |
-    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
26 
27 note: the above error was encountered while instantiating `fn f::<i32>`
28   --> $DIR/collect-in-promoted-const.rs:27:5

30 LL |     f::<i32>();
---
To only update this specific test, also pass `--test-args consts/required-consts/collect-in-promoted-const.rs`

error in revision `opt`: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/consts/required-consts/collect-in-promoted-const.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--cfg" "opt" "--check-cfg" "cfg(test,FALSE,noopt,opt)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/consts/required-consts/collect-in-promoted-const.opt" "-A" "unused" "-A" "internal_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers" "-C" "debuginfo=0" "-O"
stdout: none
--- stderr -------------------------------
error[E0080]: evaluation panicked: explicit panic
##[error]  --> /checkout/tests/ui/consts/required-consts/collect-in-promoted-const.rs:11:19
   |
LL |     const C: () = panic!(); //~ERROR evaluation panicked: explicit panic
   |                   ^^^^^^^^ evaluation of `Fail::<i32>::C` failed here

note: erroneous constant encountered
  --> /checkout/tests/ui/consts/required-consts/collect-in-promoted-const.rs:22:21
   |
LL |         let _val = &Fail::<T>::C;
   |                     ^^^^^^^^^^^^

note: the above error was encountered while instantiating `fn f::<i32>`
  --> /checkout/tests/ui/consts/required-consts/collect-in-promoted-const.rs:27:5
   |

cjgillot

One nit. r=me once CI is fixed.

View changes since this review

cjgillot · 2025-10-15T18:25:43Z

compiler/rustc_mir_transform/src/gvn.rs

+            TerminatorKind::UnwindResume
+            | TerminatorKind::UnwindTerminate(_)
+            | TerminatorKind::Drop { .. }
+            | TerminatorKind::Call { .. }
+            | TerminatorKind::TailCall { .. }
+            | TerminatorKind::Assert { .. }
+            | TerminatorKind::Yield { .. }
+            | TerminatorKind::CoroutineDrop
+            | TerminatorKind::FalseEdge { .. }
+            | TerminatorKind::FalseUnwind { .. }
+            | TerminatorKind::InlineAsm { .. } => false,


Should we move all terminators that don't write to memory to true?

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 14, 2025

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Oct 14, 2025

Auto merge of #147654 - dianqk:simplify-const-condition, r=<try>

2d02757

Simplify trivial constants in SimplifyConstCondition

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 14, 2025

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 14, 2025

dianqk force-pushed the simplify-const-condition branch from 3877315 to dcd9af0 Compare October 14, 2025 13:36

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Oct 14, 2025

Auto merge of #147654 - dianqk:simplify-const-condition, r=<try>

2b77d79

Simplify trivial constants in SimplifyConstCondition

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 14, 2025

dianqk marked this pull request as ready for review October 14, 2025 13:49

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 14, 2025

rustbot assigned davidtwco Oct 14, 2025

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 14, 2025

cjgillot reviewed Oct 15, 2025

View reviewed changes

cjgillot self-assigned this Oct 15, 2025

davidtwco removed their assignment Oct 15, 2025

mir-opt: Simplify trivial constants in SimplifyConstCondition

d5a8ab0

dianqk force-pushed the simplify-const-condition branch 2 times, most recently from e3eec7c to 64c5566 Compare October 15, 2025 15:04

GVN: Preserve derefs at unreachable and return

8f854f8

dianqk force-pushed the simplify-const-condition branch from 64c5566 to 8f854f8 Compare October 15, 2025 15:05

cjgillot reviewed Oct 15, 2025

View reviewed changes

Simplify trivial constants in SimplifyConstCondition #147654

Are you sure you want to change the base?

Simplify trivial constants in SimplifyConstCondition #147654

Conversation

dianqk commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dianqk commented Oct 14, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors bot commented Oct 14, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Oct 14, 2025

Overall result: ✅ improvements - no action needed

Uh oh!

dianqk commented Oct 14, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

rustbot commented Oct 14, 2025

Uh oh!

rustbot commented Oct 14, 2025

Uh oh!

rust-bors bot commented Oct 14, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Oct 14, 2025

Overall result: ✅ improvements - no action needed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-log-analyzer commented Oct 15, 2025

Uh oh!

cjgillot left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dianqk commented Oct 14, 2025 •

edited

Loading

cjgillot left a comment •

edited by rustbot

Loading